-
Notifications
You must be signed in to change notification settings - Fork 123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: First tentative for Plugin Mapdl Mechanism python API #3627
base: main
Are you sure you want to change the base?
Conversation
Thanks for opening a Pull Request. If you want to perform a review write a comment saying: @ansys-reviewer-bot review |
I will check it out next year. #HappyVacations |
Can't wait to discuss about it early next year when I visit you @FredAns 😃 . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good. I would also add a string dunder method __str__
, so we can do:
>>> print(mapdl.plugins)
MAPDL Plugins
----------------
DPF : feature
ABC : feature
@@ -2767,6 +2768,26 @@ def xpl(self) -> "ansXpl": | |||
self._xpl = ansXpl(self) | |||
return self._xpl | |||
|
|||
@property | |||
def plugin(self) -> "ansPlugin": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def plugin(self) -> "ansPlugin": | |
def plugin(self) -> ansPlugin: |
>>> plugin.load('PluginDPF') | ||
""" | ||
if self._plugin is None: | ||
from ansys.mapdl.core.plugin import ansPlugin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from ansys.mapdl.core.plugin import ansPlugin |
It seems to me there is no reason to delay the import. You can delete this line and then apply my previous comment (remove quotes).
Or delete the import
at the beginning and leave these lines as it is.
from ansys.mapdl.core.mapdl_grpc import MapdlGrpc | ||
|
||
if not isinstance(mapdl, MapdlGrpc): # pragma: no cover | ||
raise TypeError("Must be initialized using MapdlGrpc class") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
raise TypeError("Must be initialized using MapdlGrpc class") | |
raise TypeError("Must be initialized using an 'MapdlGrpc' object") |
from .common_grpc import ANSYS_VALUE_TYPE | ||
from .errors import MapdlRuntimeError | ||
from .misc import random_string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rather use absolute import paths:
from .common_grpc import ANSYS_VALUE_TYPE | |
from .errors import MapdlRuntimeError | |
from .misc import random_string | |
from ansys.mapdl.core.common_grpc import ANSYS_VALUE_TYPE | |
from ansys.mapdl.core.errors import MapdlRuntimeError | |
from ansys.mapdl.core.misc import random_string |
"""Return the weakly referenced instance of mapdl.""" | ||
return self._mapdl_weakref() | ||
|
||
def load(self, plugin_name: str, feature: str = "CMD") -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the default feature
is "CMD"
?
plugin_name : str | ||
Name of the plugin to load. | ||
feature : str | ||
Feature or module to activate in the plugin. | ||
|
||
Returns | ||
------- | ||
str | ||
Confirmation message about the loaded plugin. | ||
|
||
Raises | ||
------ | ||
PluginLoadError | ||
If the plugin fails to load. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency, I prefer only one tab.
plugin_name : str | |
Name of the plugin to load. | |
feature : str | |
Feature or module to activate in the plugin. | |
Returns | |
------- | |
str | |
Confirmation message about the loaded plugin. | |
Raises | |
------ | |
PluginLoadError | |
If the plugin fails to load. | |
plugin_name : str | |
Name of the plugin to load. | |
feature : str | |
Feature or module to activate in the plugin. | |
Returns | |
------- | |
str | |
Confirmation message about the loaded plugin. | |
Raises | |
------ | |
PluginLoadError | |
If the plugin fails to load. |
command = f"*PLUG,LOAD,{plugin_name},{feature}" | ||
response = self._mapdl.run(command) | ||
if "error" in response.lower(): | ||
raise PluginLoadError( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should create this type of exception in ansys.mapdl.core.errors
, ideally inheritate from MapdlException
or MapdlRuntimeError
.
raise PluginLoadError( | ||
f"Failed to load plugin '{plugin_name}' with feature '{feature}'." | ||
) | ||
return f"Plugin '{plugin_name}' with feature '{feature}' loaded successfully." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you not returned the original MAPDL response response
?
response = self._mapdl.run(command) | ||
if "error" in response.lower(): | ||
raise PluginUnloadError(f"Failed to unload plugin '{plugin_name}'.") | ||
return f"Plugin '{plugin_name}' unloaded successfully." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same. Why not default MAPDL response?
command = "*PLUG,LIST" | ||
response = self._mapdl.run(command) | ||
if "error" in response.lower(): | ||
raise RuntimeError("Failed to retrieve the list of loaded plugins.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
raise RuntimeError("Failed to retrieve the list of loaded plugins.") | |
from ansys.mapdl.core.errors import MapdlRuntimeError | |
raise MapdlRuntimeError("Failed to retrieve the list of loaded plugins.") |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3627 +/- ##
==========================================
- Coverage 87.09% 86.88% -0.22%
==========================================
Files 187 188 +1
Lines 14694 14736 +42
==========================================
+ Hits 12798 12803 +5
- Misses 1896 1933 +37 |
Description
**There's now a new command in MAPDL, called *PLUG, to allow the loading of external dynamic libraries, to dynamically add custom commands and features in MAPDL ( for the time of the session). There are a few plugins already shipped with MAPDL ( The gRPC server lib and the DPF Plugin) we can load and use this way.
I've copy/paste what was done for the XPL API implementation **
Issue linked
There's a TFS User Story for this action. But not already a Github ID.
Checklist
draft
if it is not ready to be reviewed yet.feat: adding new MAPDL command
)